MoveSCU implementation #108
Conversation
There was a problem hiding this comment.
Thank you for working on this. The expectations of use are still a bit too off for the tool to enter the ecosystem. I have gathered the related comments below.
To create a query DICOM object file, you can follow the advice from DCMTK regarding findscu. This text file:
# query patient names and IDs
(0008,0052) CS [PATIENT] # QueryRetrieveLevel
(0010,0010) PN [] # PatientsName
(0010,0020) LO [P123] # PatientID
can be converted to a DICOM file with the DCMTK command dump2dcm. The file would then serve as input to this tool (in this example, to move all images of patient with the ID P123).
Other minor concerns are that this is still missing a readme file.
I would appreciate it if you could look into this in some of your spare time. Otherwise, I can put it in my backlog.
| /// the called AE title | ||
| #[structopt(long = "called-ae-title", default_value = "ANY-SCP")] | ||
| called_ae_title: String, | ||
|
|
There was a problem hiding this comment.
| /// the maximum PDU length |
| } = App::from_args(); | ||
|
|
||
| if verbose { | ||
| println!("Establishing association with '{}'...", &addr); |
There was a problem hiding this comment.
println! and other macros already borrow arguments implicitly.
| println!("Establishing association with '{}'...", &addr); | |
| println!("Establishing association with '{}'...", addr); |
| /// the DICOM file to store | ||
| file: PathBuf, | ||
| /// the C-MOVE destination | ||
| #[structopt(short = "mo", long = "move-destination", default_value = "")] |
There was a problem hiding this comment.
Short arguments longer than 1 character are not supported by clap.
In addition, I would say that empty string as the default value does not make much sense. The user has to provide a move destination, otherwise it is unclear where it should go.
| #[structopt(short = "mo", long = "move-destination", default_value = "")] | |
| #[structopt(long = "move-destination")] |
| let ts = TransferSyntaxRegistry | ||
| .get(&transfer_syntax) | ||
| .expect("Poorly negotiated transfer syntax"); | ||
|
|
There was a problem hiding this comment.
As of the established version of the association API, the client association no longer provides presentation_context_id, because it keeps all accepted presentation contexts.
One would need to pick one of the accepted presentation contexts and use the id of that one in subsequent interactions.
| let pc_selected = if let Some(pc) = scu | |
| .presentation_contexts() | |
| .iter() | |
| .find(|pc| transfer_syntax == &pc.transfer_syntax) | |
| { | |
| pc.clone() | |
| } else { | |
| eprintln!("Could not choose a transfer syntax"); | |
| let _ = scu.abort(); | |
| std::process::exit(-2); | |
| }; | |
| /// verbose mode | ||
| #[structopt(short = "v")] | ||
| verbose: bool, | ||
| /// the DICOM file to store |
There was a problem hiding this comment.
This made sense in the store SCU, but not anymore in the move SCU, since the application might not have the objects it wants to move in the first place. Instead, we can follow the approach done by other tools (e.g. DCMTK's movescu), and declare that the input file represents a query object.
| /// the DICOM file to store | |
| /// the DICOM query file |
| let sop_instance_uid = &meta.media_storage_sop_instance_uid; | ||
| let transfer_syntax = "1.2.840.10008.1.2"; | ||
|
|
||
| let retrieve_level = "STUDY "; |
There was a problem hiding this comment.
I had overlooked this until I actually tried to run the application. The retrieval level should be fetched from the query file (QueryRetrieveLevel (0008,0052)), and only the attributes listed therein should be considered. The study instance UID or the series instance UID might not even be present.
| obj | ||
| } | ||
|
|
||
| fn create_iod( |
There was a problem hiding this comment.
Once we're upfront that the input file is already a query file, there does not seem to be a reason for this function to exist other than to map one query object (from the file) into another query object. It should probably just be removed.
|
I'm afraid that this has diverged too much from the base project. It will need to be rebuilt from scratch, maybe after #463 so that we can couple the move SCU with a local store SCP. |
A command line interface for C-MOVE SCU.